Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Feature][Java/Spring] Support Discriminator Based OneOf Interface #11650

Conversation

cachescrubber
Copy link
Contributor

@cachescrubber cachescrubber commented Feb 18, 2022

Support Discriminator Based OneOf Interface

Superseeds

Also Fixes

Brings basic discriminator based OneOf Support to the JavaSpring generator. It generates a marker Interface which the oneOf Models then implement. Technically it requires Jackson based models and is implemented using @JsonTypeInfo and @JsonSubTypes based using a discriminator property.

The relevant Code ist moved to AbstractJavaCodegen and aligned with the other Java based generators in the hierarchy.

The oneOf Support is implemented by utilizing the x-implements vendor extension.

Currently It is enabled in the SpringCodegen only:

useOneOfInterfaces = true;
legacyDiscriminatorBehavior = false;

Synopsis

@JsonTypeInfo(use = JsonTypeInfo.Id.NAME, include = JsonTypeInfo.As.EXISTING_PROPERTY, property = "@type", visible = true)
@JsonSubTypes({
  @JsonSubTypes.Type(value = Foo.class, name = "Foo"),
  @JsonSubTypes.Type(value = FooRef.class, name = "FooRef")
})

@Generated(value = "org.openapitools.codegen.languages.SpringCodegen")
public interface FooRefOrValue {
    public String getAtType();
}

PR checklist

  • [ x] Read the contribution guidelines.
  • [ x] Pull Request title clearly describes the work in the pull request and Pull Request description provides details about how to validate the work. Missing information here may result in delayed response from the community.
  • [x ] Run the following to build the project and update samples:
    ./mvnw clean package 
    ./bin/generate-samples.sh
    ./bin/utils/export_docs_generators.sh
    
    Commit all changed files.
    This is important, as CI jobs will verify all generator outputs of your HEAD commit as it would merge with master.
    These must match the expectations made by your contribution.
    You may regenerate an individual generator by passing the relevant config(s) as an argument to the script, for example ./bin/generate-samples.sh bin/configs/java*.
    For Windows users, please run the script in Git BASH.
  • [ x] File the PR against the correct branch: master (5.3.0), 6.0.x
  • [ x] If your PR is targeting a particular programming language, @mention the technical committee members, so they are more likely to review the pull request.

alexsuperdev and others added 30 commits March 22, 2020 11:15
added x-is-one-of-interface extension for oneOf interface in mustache
template
fixed name of model from UNKNOWN_BASE_TYPE to right one in api: operationId + OneOf

Fix OpenAPITools#5381
parcelableModel is not required
removed not needed methods
catch NPE cases in preprocessOpenAPI
updated samples
fixed generation of oneOf Models
addOneOfInterfaceModel only for cases when useOneOfInterfaces is true and for spring
…rator into spring_fix_5381

� Conflicts:
�	modules/openapi-generator/src/main/java/org/openapitools/codegen/DefaultCodegen.java
spring: fixed use of oneOf Models in API
implementing oneOf for spring lib overriding methods with different behavior from default
added x-is-one-of-interface extension for oneOf interface in mustache
template
fixed name of model from UNKNOWN_BASE_TYPE to right one in api: operationId + OneOf

Fix OpenAPITools#5381
removed not needed methods

Fix OpenAPITools#5381
fixed generation of oneOf Models

Fix OpenAPITools#5381
addOneOfInterfaceModel only for cases when useOneOfInterfaces is true and for spring

Fix OpenAPITools#5381
NPE fix for tests
fixed handing of composed schema with array
fixed NPE in addOneOfInterfaceModel
fixed generation of oneOf models with descriminator
# Conflicts:
#	modules/openapi-generator/src/main/java/org/openapitools/codegen/DefaultCodegen.java
# Conflicts:
#	modules/openapi-generator/src/test/java/org/openapitools/codegen/java/spring/SpringCodegenTest.java
# Conflicts:
#	modules/openapi-generator/src/main/java/org/openapitools/codegen/DefaultCodegen.java
#	modules/openapi-generator/src/main/java/org/openapitools/codegen/languages/SpringCodegen.java
# Conflicts:
#	modules/openapi-generator/src/main/resources/JavaSpring/pojo.mustache
#	modules/openapi-generator/src/test/java/org/openapitools/codegen/java/spring/SpringCodegenTest.java

@Schema(name = "EntityRef", description = "Entity reference schema to be use for all entityRef class.")
@JsonTypeInfo(use = JsonTypeInfo.Id.NAME, include = JsonTypeInfo.As.EXISTING_PROPERTY, property = "@type", visible = true)
@JsonSubTypes({
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Example for inheritance using discriminator



@Generated(value = "org.openapitools.codegen.languages.SpringCodegen")
public class FooRef extends EntityRef implements FooRefOrValue {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Example: Inherit from EntityRef class using discriminator an implementing the oneOf Interface.

})

@Generated(value = "org.openapitools.codegen.languages.SpringCodegen")
public interface FooRefOrValue {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The OneOf Interface

Copy link
Contributor

@welshm welshm left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Makes sense overall - I do wonder how nested oneOf will work

@jburgess
Copy link
Contributor

jburgess commented Mar 1, 2022

@cachescrubber Thank you so much for picking this up and keeping it up-to-date with the base branch! Initial testing looks good on this PR.

@wing328 Can we get this in a milestone?

@cachescrubber cachescrubber mentioned this pull request Mar 6, 2022
5 tasks
@@ -46,7 +46,7 @@ import javax.annotation.Generated;
{{>enumOuterClass}}
{{/isEnum}}
{{^isEnum}}
{{>pojo}}
{{#vendorExtensions.x-is-one-of-interface}}{{>oneof_interface}}{{/vendorExtensions.x-is-one-of-interface}}{{^vendorExtensions.x-is-one-of-interface}}{{>pojo}}{{/vendorExtensions.x-is-one-of-interface}}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this still necessary after #11784 ?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, very much so. It the model entity is a one-of-interface, a public interface MyModel is generated instead of a public class myModel. Or do I miss something?

@wing328
Copy link
Member

wing328 commented Mar 8, 2022

@cachescrubber thanks for the PR. Here are something we discussed before and put it here so that everyone is on the same page:

  • implementation by this PR does not support primitive type, e.g.
    IntOrString:
      oneOf:
        - type: string
        - type: integer   
  • oneOf does not imply inheritance

# Conflicts:
#	modules/openapi-generator/src/main/resources/JavaSpring/pojo.mustache
@wing328 wing328 added this to the 6.0.0 milestone Mar 16, 2022
@wing328
Copy link
Member

wing328 commented Mar 16, 2022

Also just to add that java (okhttp-gson, jersey2, native) client generator has a different implementation of oneOf, anyOf.

We can give this PR (implementation) a try and collect feedback from our users.

@cachescrubber
Copy link
Contributor Author

@wing328. Thanks for the review and merging!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants